Skip to content

Trim runtime deps: make Nashorn optional, bulk up JS filter tests#1775

Merged
tfenne merged 1 commit into
devfrom
tf_update_deps
Apr 25, 2026
Merged

Trim runtime deps: make Nashorn optional, bulk up JS filter tests#1775
tfenne merged 1 commit into
devfrom
tf_update_deps

Conversation

@tfenne
Copy link
Copy Markdown
Member

@tfenne tfenne commented Apr 25, 2026

Summary

Slim htsjdk's published runtime classpath by making the JavaScript engine an opt-in dependency, with a meaningful test bulk-up around the change.

Net effect for v5.0.0 consumers: 6 fewer jars on the runtime classpath (nashorn-core + 5× ASM transitives, ~2.5 MB) and one less misleading "direct dependency" in the published POM.

⚠️ Breaking change

Consumers using JavascriptSamRecordFilter or JavascriptVariantFilter must now add a JSR-223 "js" engine to their own runtime classpath. The recommended choice is OpenJDK Nashorn:

// Gradle
runtimeOnly 'org.openjdk.nashorn:nashorn-core:15.7'
<!-- Maven -->
<dependency>
  <groupId>org.openjdk.nashorn</groupId>
  <artifactId>nashorn-core</artifactId>
  <version>15.7</version>
  <scope>runtime</scope>
</dependency>

If no engine is on the classpath, the filter constructors now throw RuntimeScriptException with a multi-line, fully self-describing message that names the calling filter class and prints both Gradle and Maven coordinates — the failure is supposed to be impossible to misread.

This will also be documented in the dedicated v5.0.0 CHANGELOG / breaking-changes PR.

What's in the PR

build.gradle

  • Nashorn 15.4 → 15.7, moved from implementation to compileOnly. Also added as testImplementation so the project's own JS-filter tests still run.
  • Removed the direct commons-logging:1.3.0 declaration. htsjdk does not use commons-logging itself; it was declared only to bump the version that commons-jexl:2.1.1 transitively pulls (1.1.1, from 2007). Replaced with a Gradle dependency constraint, so the published POM advertises the version pin in <dependencyManagement> rather than as a real <dependency>. Same runtime behavior, more honest metadata.

AbstractJavascriptFilter

  • Replaced the prior "Do you use the SUN/Oracle Java Runtime?" error with a multi-line, fully self-describing message via a new package-private noJsEngineMessage(String) helper.

JavascriptSamRecordFilter / JavascriptVariantFilter

  • Class Javadoc rewritten with a clear summary, a small inline usage example, and a "Runtime requirement" callout pointing at the nashorn-core artifact.

Test bulk-up (prep work for the engine swap, but it's good in its own right)

  • Deleted 4 checked-in JavaScript fixtures (samFilter01.js, samFilter02.js, variantFilter01.js, variantFilter02.js).
  • Rewrote JavascriptSamRecordFilterTest and JavascriptVariantFilterTest as many small, focused tests using inline String scripts and programmatically-built SAMFileHeader / VCFHeader / SAMRecord / VariantContext fixtures. 46 new test cases vs the previous 4 effective cases. Coverage now includes:
    • All three constructors (File / String / Reader)
    • Return-type semantics (null/undefined/boolean/0/1/2/-1/1.0/string)
    • Bindings reachability (record/variant/header) and subclass record-key override (SAM)
    • Error paths (malformed JS, throwing scripts)
    • filterOut(first, second) AND-semantics (SAM only)
    • Multi-record reuse / per-record bindings refresh
    • The no-engine error message stays actionable (asserts it names the calling class, the artifact, and shows both Gradle and Maven snippets)

Test plan

  • ./gradlew compileJava compileTestJava — passes
  • ./gradlew test21,963 / 21,963 pass
  • ./gradlew spotlessCheck — clean
  • ./gradlew dependencies --configuration runtimeClasspath — confirms nashorn-core and the 5 ASM artifacts are gone; commons-logging now shows under jexl as 1.1.1 -> 1.3.0 (transitive) plus a (c) constraint marker rather than as a top-level dep.
  • CI: confirm test, formatCheck, spotBugs jobs green

Final runtime tree consumers will see (post-merge)

htsjdk
├── org.apache.commons:commons-jexl:2.1.1                (api)
│   └── commons-logging:commons-logging:1.1.1 → 1.3.0    (bumped by constraint)
├── com.fulcrumgenomics:jlibdeflate:0.1.0
├── org.xerial.snappy:snappy-java:1.1.10.5
├── org.apache.commons:commons-compress:1.26.0
│   ├── commons-io:commons-io:2.15.1
│   └── org.apache.commons:commons-lang3:3.14.0
├── org.tukaani:xz:1.9
├── org.json:json:20231013
└── commons-logging:commons-logging:1.3.0 (c)            (constraint, not a real dep)

8 declared deps + 1 constraint, vs 14 before this PR (across this branch + #1774).

Slim htsjdk's published runtime classpath by making the JavaScript
engine an opt-in dependency, and clean up around the change. Net effect
for consumers of the v5.0.0 artifact: 6 fewer jars on the runtime
classpath (nashorn-core + 5 ASM transitives, ~2.5 MB) and one less
"misleading direct dependency" in the published POM.

** BREAKING CHANGE **

Consumers using htsjdk.samtools.filter.JavascriptSamRecordFilter or
htsjdk.variant.variantcontext.filter.JavascriptVariantFilter must now
add a JSR-223 "js" engine to their own runtime classpath. The
recommended choice is OpenJDK Nashorn:

    Gradle:  runtimeOnly 'org.openjdk.nashorn:nashorn-core:15.7'

    Maven:   <dependency>
               <groupId>org.openjdk.nashorn</groupId>
               <artifactId>nashorn-core</artifactId>
               <version>15.7</version>
               <scope>runtime</scope>
             </dependency>

If no engine is on the classpath at runtime, AbstractJavascriptFilter's
constructor now throws a RuntimeScriptException whose message names the
calling filter class and prints both Gradle and Maven coordinates, so
the failure is fully self-describing.

** Changes **

build.gradle:
- Nashorn moved from `implementation` to `compileOnly` (and bumped
  15.4 -> 15.7). It also appears as `testImplementation` so the
  project's own test suite still has an engine available.
- Removed the direct `commons-logging:1.3.0` declaration. htsjdk does
  not use commons-logging itself; it was only declared to bump the
  version that commons-jexl 2.1.1 transitively pulls (1.1.1, from 2007).
  Replaced with a Gradle dependency constraint, so the published POM
  advertises the version pin in <dependencyManagement> rather than as
  a real direct dep. Same runtime behavior, more honest metadata.

src/main/java/htsjdk/samtools/filter/AbstractJavascriptFilter.java:
- Replaced the prior "Do you use the SUN/Oracle Java Runtime?" error
  with a multi-line, fully self-describing message via a new
  package-private noJsEngineMessage(String) helper.

src/main/java/htsjdk/samtools/filter/JavascriptSamRecordFilter.java
src/main/java/htsjdk/variant/variantcontext/filter/JavascriptVariantFilter.java:
- Class Javadoc rewritten with a clear summary, a small inline usage
  example, and a "Runtime requirement" callout pointing at the
  nashorn-core artifact coordinates.

Test changes (separate prep work, kept here so the entire JS-filter
surface lands in one PR):
- Deleted 4 checked-in JavaScript test fixtures
  (samFilter01.js, samFilter02.js, variantFilter01.js, variantFilter02.js).
- Rewrote both JavascriptSamRecordFilterTest and JavascriptVariantFilterTest
  as many small, focused tests using inline String scripts and
  programmatically-built SAMFileHeader / VCFHeader / SAMRecord /
  VariantContext fixtures. New coverage: all three constructors,
  return-type semantics (null/undefined/boolean/integer/double/string),
  bindings reachability for both `record`/`variant` and `header`,
  subclass record-key override (SAM), error paths (malformed JS,
  exception-throwing scripts), filterOut(first, second) AND-semantics
  (SAM), multi-record reuse, and one assertion that the no-engine
  error message stays actionable. 46 new test cases in total.

** Verification **
- ./gradlew test: 21,963 / 21,963 pass.
- ./gradlew dependencies --configuration runtimeClasspath: confirms
  nashorn-core and the 5 ASM artifacts are gone; commons-logging now
  shows under jexl as 1.1.1 -> 1.3.0 (transitive) plus a (c) constraint
  marker rather than as a top-level dep.

The breaking change will be called out in the dedicated v5.0.0
CHANGELOG / breaking-changes PR alongside the other v5.0.0 changes.
@tfenne tfenne merged commit 9fae270 into dev Apr 25, 2026
4 checks passed
@tfenne tfenne deleted the tf_update_deps branch April 25, 2026 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant